Skip to content

fix: correct touchscreen right-click menu interaction#1482

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
18202781743:master
Mar 6, 2026
Merged

fix: correct touchscreen right-click menu interaction#1482
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
18202781743:master

Conversation

@18202781743
Copy link
Contributor

@18202781743 18202781743 commented Mar 6, 2026

Changed the right-click menu trigger from onTapped to onPressedChanged
in two TapHandler instances within NotifyViewDelegate.qml. The previous
implementation using onTapped caused incorrect behavior on touchscreen
devices where the right-click menu would appear and immediately
disappear. The new implementation triggers the menu when the button is
pressed (pressed becomes true) rather than when the tap is completed,
ensuring proper menu display on touchscreen devices.

Log: Fixed touchscreen right-click menu interaction issue where menus
would not stay open

Influence:

  1. Test right-click menu behavior on touchscreen devices
  2. Verify menu appears and remains visible when right-clicking
    notifications
  3. Test that menu functionality remains unchanged for mouse users
  4. Verify menu positioning is correct relative to the touch point
  5. Test interaction with different notification types in the
    notification center

fix: 修正触摸屏右键菜单交互问题

将 NotifyViewDelegate.qml 中两个 TapHandler 实例的右键菜单触发方式从
onTapped 改为 onPressedChanged。之前的实现使用 onTapped 会导致在触摸屏设
备上右键菜单出现后立即消失的问题。新的实现方式在按钮按下时(pressed 变为
true)触发菜单显示,而不是在点击完成时触发,确保在触摸屏设备上菜单能正确
显示。

Log: 修复触摸屏右键菜单交互问题,解决菜单无法保持打开状态的问题

Influence:

  1. 在触摸屏设备上测试右键菜单行为
  2. 验证右键点击通知时菜单出现并保持可见
  3. 测试鼠标用户的菜单功能是否保持不变
  4. 验证菜单位置相对于触摸点的定位是否正确
  5. 测试通知中心中不同类型通知的交互

PMS: BUG-352017

Summary by Sourcery

Bug Fixes:

  • Resolve issue where right-click context menus on notifications would immediately close on touchscreen devices by triggering them on press instead of tap completion.

Changed the right-click menu trigger from onTapped to onPressedChanged
in two TapHandler instances within NotifyViewDelegate.qml. The previous
implementation using onTapped caused incorrect behavior on touchscreen
devices where the right-click menu would appear and immediately
disappear. The new implementation triggers the menu when the button is
pressed (pressed becomes true) rather than when the tap is completed,
ensuring proper menu display on touchscreen devices.

Log: Fixed touchscreen right-click menu interaction issue where menus
would not stay open

Influence:
1. Test right-click menu behavior on touchscreen devices
2. Verify menu appears and remains visible when right-clicking
notifications
3. Test that menu functionality remains unchanged for mouse users
4. Verify menu positioning is correct relative to the touch point
5. Test interaction with different notification types in the
notification center

fix: 修正触摸屏右键菜单交互问题

将 NotifyViewDelegate.qml 中两个 TapHandler 实例的右键菜单触发方式从
onTapped 改为 onPressedChanged。之前的实现使用 onTapped 会导致在触摸屏设
备上右键菜单出现后立即消失的问题。新的实现方式在按钮按下时(pressed 变为
true)触发菜单显示,而不是在点击完成时触发,确保在触摸屏设备上菜单能正确
显示。

Log: 修复触摸屏右键菜单交互问题,解决菜单无法保持打开状态的问题

Influence:
1. 在触摸屏设备上测试右键菜单行为
2. 验证右键点击通知时菜单出现并保持可见
3. 测试鼠标用户的菜单功能是否保持不变
4. 验证菜单位置相对于触摸点的定位是否正确
5. 测试通知中心中不同类型通知的交互

PMS: BUG-352017
@sourcery-ai
Copy link

sourcery-ai bot commented Mar 6, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Switches right-click notification menu handling in NotifyViewDelegate.qml from tap-completion to press-state changes so that context menus stay open on touchscreens, while preserving mouse behavior and positioning.

Sequence diagram for updated touchscreen right-click menu handling

sequenceDiagram
    actor User
    participant Touchscreen
    participant NotifyViewDelegate
    participant TapHandler
    participant ContextMenu

    User->>Touchscreen: Long-press or right-tap notification
    Touchscreen->>NotifyViewDelegate: Pointer event with right button
    NotifyViewDelegate->>TapHandler: Handle event (acceptedButtons Qt.RightButton)
    TapHandler-->>TapHandler: pressed becomes true
    TapHandler->>TapHandler: onPressedChanged()
    TapHandler->>NotifyViewDelegate: setting(point.position)
    NotifyViewDelegate->>ContextMenu: showAt(position)
    ContextMenu-->>User: Menu remains visible until dismissed
Loading

File-Level Changes

Change Details Files
Trigger right-click context menu on press state instead of tap completion for notification items.
  • Replaced TapHandler onTapped callbacks with onPressedChanged handlers for right-click interactions.
  • Added pressed-state guard so the context menu is only opened when pressed becomes true, avoiding toggle-on-release behavior.
  • Changed event usage to derive the menu position from the handler’s point object before calling the menu-setting function.
panels/notification/center/NotifyViewDelegate.qml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码修改涉及到了 QML 中 TapHandler 事件处理的逻辑变更。以下是对这段 diff 的详细审查,包括语法逻辑、代码质量、代码性能和代码安全方面的分析:

1. 语法逻辑分析

  • 变更内容:将右键点击的触发方式从 onTapped 修改为 onPressedChanged,并配合 pressed 属性判断。
  • 逻辑影响
    • 原逻辑 (onTapped):只有当用户完成点击(按下并抬起)动作时才会触发 setting(pos)。这是标准的点击行为。
    • 新逻辑 (onPressedChanged):当用户按下鼠标右键(pressed 变为 true)的瞬间立即触发 setting(pos)。这意味着用户不需要抬起手指/鼠标,菜单就会弹出。
  • 潜在问题:这种修改改变了交互行为。通常右键菜单是在“按下并抬起”或者“按下”时弹出的(取决于操作系统习惯,Qt/Windows 通常在按下时弹出,macOS 通常在抬起时弹出)。如果原设计意图是“点击后弹出”,新代码会让响应变得非常灵敏(甚至在用户可能想拖拽取消操作时就已经弹出了)。

2. 代码质量

  • 变量作用域:新代码使用了 let pos = point.position。这里的 pointTapHandleronPressedChanged 信号中隐式提供的上下文属性(代表触发点)。虽然在 QML 中这种写法是合法的,但相比原代码中 onTapped(eventPoint, button) 显式接收参数,依赖隐式上下文属性可能会降低代码的可读性和可维护性,尤其是在复杂的嵌套组件中。
  • 一致性:diff 中显示有两处相同的修改(分别在不同的 DelegateChooser 分支中),说明这是一次统一的交互调整,这一点做得很好,保证了 UI 行为的一致性。

3. 代码性能

  • 性能影响onPressedChanged 在按下和抬起时都会触发。代码中增加了 if (pressed) 判断,确保只在按下时执行逻辑。这避免了抬起时的无效计算。
  • 对比onTapped 本身只触发一次。新逻辑虽然多了一个 if 判断,但性能开销微乎其微,可以忽略不计。

4. 代码安全

  • 空值风险point.positionpressedtrue 时通常是有效的,但如果 TapHandler 处于某些边缘状态,理论上可能存在风险。不过,在 pressedtrue 的回调中,point 对象通常是安全的。
  • 逻辑安全:如果 setting(pos) 函数内部没有防抖或状态检查,且用户快速连续点击,可能会导致菜单状态混乱。但这一点与原代码风险一致,并非新引入的问题。

改进建议

  1. 交互意图确认

    • 如果目的是实现“按下即弹出菜单”(类似 Windows 原生右键菜单行为),目前的修改是正确的。
    • 如果目的是修复某些点击不灵敏的问题,或者保持原有行为,建议回退或重新考虑。
    • 建议:明确产品需求对右键菜单弹出时机的具体要求。
  2. 代码可读性优化

    • 虽然使用了隐式的 point,但为了代码更清晰,可以考虑利用 TapHandlerpoint 属性(它是当前主要交互点的别名),或者在注释中说明 point 的来源。
    • 如果不需要 pos 变量做额外处理,可以直接传递:setting(point.position)
  3. 代码优化示例
    如果确认需要“按下即触发”,目前的写法是 QML 中处理此类事件的标准方式。不过,为了更符合 QML 风格,可以稍微精简:

    TapHandler {
        acceptedButtons: Qt.RightButton
        // onPressedChanged 会在按下和抬起时触发
        onPressedChanged: {
            if (pressed) {
                // 直接使用 point,无需额外声明 let pos(除非 setting 需要特定格式)
                setting(point.position)
            }
        }
    }
  4. 潜在的逻辑冲突

    • 检查 setting 函数内部是否处理了菜单的关闭逻辑。如果在菜单打开状态下再次按下右键,行为是否符合预期?

总结

这段代码在语法上是正确的,性能上没有问题。主要的变更在于交互逻辑的时机(从“抬起触发”变为“按下触发”)。建议根据产品的具体交互规范决定是否采纳此修改。如果是为了模仿桌面系统的原生右键菜单行为(通常是按下时弹出),这是一个合理的改进。

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, mhduiy

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@18202781743
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Mar 6, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 4429925 into linuxdeepin:master Mar 6, 2026
9 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants